Fix CIDR whitelist entries being discarded on hot reload - #4
Merged
Conversation
A CIDR range added to /logs/whitelist.txt worked until the next reload,
then silently stopped:
Ignoring invalid whitelisted IP entry in /logs/whitelist.txt: 10.42.0.0/16
Ignoring invalid whitelisted IP entry in /logs/whitelist.txt: 129.215.0.0/16
The list compiler had two copies. docker-entrypoint.sh learned to route
ranges into a `geo` map (273c827, 1ae044e), but health-monitor.sh kept a
private pre-CIDR copy and recompiled the whitelist with it on every file
change. So ranges were dropped on reload, and because the monitor never
wrote whitelisted-cidrs.map at all, a range added after boot never took
effect without a container restart.
Both scripts now source ip-maps.sh, so they cannot drift again. The test
suite includes static wiring assertions that fail the build if they do --
unit tests alone would not have caught the original bug, since each copy
of the compiler was individually correct.
Also fixes a second defect exposed by the same asymmetry: the auto-block
exemption used an exact string match, so an address inside a whitelisted
CIDR could trip a probe pattern and be auto-blocked despite its whole
network being whitelisted, leaving the two map files disagreeing about
the same address. It is now range-aware.
Range membership is computed by division rather than 2^(32-bits): BusyBox
can be built without libm, and there `^` aborts with "Math support is not
compiled in", which made every in-range address read as out-of-range --
silently, and only on the alpine runtime image.
46 checks, run at image build time so a regression fails `docker build`
rather than surfacing later as a cache bypass that quietly doesn't happen.
Verified under BusyBox ash with a libm-less awk, dash, and bash in POSIX
mode.
README documents range support, the fact that rejected lines are logged,
and X-Force-Refresh, which until now existed only as a code comment.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y8ZAhRM7gh8arDx9SuSWcT
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
A CIDR range added to
/logs/whitelist.txtworks until the next hot reload, then silently stops:Root cause
The list compiler had two copies.
docker-entrypoint.shcompiles the lists once at container start;health-monitor.shrecompiles them on every change toblocked.txt/whitelist.txt, then reloads nginx.When the entrypoint learned to route CIDR ranges into a
geomap (273c827, 1ae044e), the monitor's private copy of the compiler did not. So:whitelisted-cidrs.mapat all, a range added after boot never takes effect without restarting the container.That is why both
129.215.0.0/16(a VPN pool) and10.42.0.0/16(the Rancher/Canal pod network the VFBquery warmup tool runs from) were being rejected.The fix
Both scripts now source a shared
ip-maps.sh, so they cannot drift again. Drift was the bug, so a third copy of the function would have been the wrong shape of fix.The test suite includes static wiring assertions that grep both callers and fail if either stops sourcing the library, reintroduces a private copy of the compiler, or stops passing the CIDR map. Unit tests alone would not have caught the original bug — each copy of the compiler was individually correct.
Second defect, same asymmetry
The auto-block exemption used an exact string match (
is_ip_listed), so an address inside a whitelisted CIDR could trip a probe pattern and get auto-blocked despite the operator having whitelisted its whole network — after which the two map files disagree about the same address. It is now range-aware (is_ip_whitelisted).IPv6 ranges are not matched arithmetically (no bitwise ops in BusyBox awk, and
::compression makes textual prefix matching unsafe). Instead, if any IPv6 range is whitelisted at all, no IPv6 address is auto-blocked — erring towards leaving a scanner unblocked rather than locking out a trusted host, which is the right way round here: the probe filter still returns 403 either way, auto-blocking only saves the work of matching it.BusyBox finding
Range membership is computed by division rather than
2^(32-bits). BusyBox can be built without libm, and there^aborts withMath support is not compiled in— which made every in-range address read as out-of-range. This was caught only by running the suite under the runtime image's own awk; it would have shipped silently and failed only in production.Tests
test/ip-maps-test.sh, 46 checks: line classification, whitelist compilation (dedupe, sort, CRLF, uppercase hex, comments, junk, and an explicit assertion that129.215.0.0/16lands in the geo map and is not reported invalid), blocklist compilation, 15 IPv4 range-membership cases including/0,/32and both boundaries, auto-block exemption, and the wiring assertions.Run at image build time, so a regression fails
docker buildrather than surfacing hours later as a cache bypass that quietly doesn't happen. Gating on the build rather than a new workflow job was deliberate — CI already runsdocker build.Verified passing under BusyBox ash with a libm-less BusyBox awk, dash, bash in POSIX mode, and BusyBox ash with system awk. Also verified end-to-end against the real
health-monitor.sh: appending the two ranges to a live whitelist now produces awhitelisted-cidrs.mapcontaining both, with no "Ignoring invalid" warning.README
Documents that the whitelist accepts ranges and why that matters (a VPN or pod network reassigned per session can be whitelisted once), that the blocklist deliberately does not, that rejected lines are reported on the container log, and — for the first time — the
X-Force-Refreshheader, which previously existed only as a code comment. Includes thex-cache-status: BYPASSvsHITcheck for confirming a bypass actually happened.